Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[12팀 배성규] [Chapter 2-2] 디자인 패턴과 함수형 프로그래밍 #6

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

pangkyu
Copy link

@pangkyu pangkyu commented Jan 13, 2025

과제 체크포인트

기본과제

  • React의 hook 이해하기

  • 함수형 프로그래밍에 대한 이해

  • Component에서 비즈니스 로직을 분리하기

  • 비즈니스 로직에서 특정 엔티티만 다루는 계산을 분리하기

  • Component에서 사용되는 Data가 아닌 로직들은 hook으로 옮겨졌나요?

  • 주어진 hook의 책임에 맞도록 코드가 분리가 되었나요?

  • 계산함수는 순수함수로 작성이 되었나요?

심화과제

  • 뷰데이터와 엔티티데이터의 분리에 대한 이해

  • 엔티티 -> 리파지토리 -> 유즈케이스 -> UI 계층에 대한 이해

  • Component에서 사용되는 Data가 아닌 로직들은 hook으로 옮겨졌나요?

  • 주어진 hook의 책임에 맞도록 코드가 분리가 되었나요?

  • 계산함수는 순수함수로 작성이 되었나요?

  • 특정 Entitiy만 다루는 함수는 분리되어 있나요?

  • 특정 Entitiy만 다루는 Component와 UI를 다루는 Component는 분리되어 있나요?

  • 데이터 흐름에 맞는 계층구조를 이루고 의존성이 맞게 작성이 되었나요?

과제 셀프회고

과제에서 좋았던 부분

회사에서도 코드를 정리해야할 일이 있었는데, 이번주 과제를 진행하면서 도움이 되는 인사이트를 많이 얻었습니다.

과제를 하면서 새롭게 알게된 점

리팩토링 과정은 끝이 없다.. :(

과제를 진행하면서 아직 애매하게 잘 모르겠다 하는 점, 혹은 뭔가 잘 안되서 아쉬운 것들

  • useLocalStorage
    해당 훅을 먼저 작성하고 코드를 정리하려고 했었는데, 생각보다 시간을 많이 잡아먹어서 다른 것에 구현 시간을 분배하지 못했습니다.
  • 테스트 코드
    테스트 코드 작성하는 게 아직 익숙하지가 않아서 좀 더 학습이 필요하다고 생각합니다.

리뷰 받고 싶은 내용이나 궁금한 것에 대한 질문

컴포넌트를 분리하고 데이터를 내리다보면 과도하게 props drilling이 일어나지 않을까?라는 생각이 듭니다. 이런 과도한 props drilling을 해결하기 위해서 전역변수나 커스텀 훅 등의 해결방법을 사용한다고 알고 있습니다.
해결 방법을 결정하실 때 어떤 방법으로 처리하시는 것을 선호하시는 지와 그 이유가 궁금합니다.
++ 그리고 커스텀 훅으로 뺀다고 생각했는데 가끔 props drilling이 남아있는 경우가 있는데 그때 문제 해결을 어떤 방식으로 접근해야할 지 조언주시면 감사드리겠습니다

@pangkyu pangkyu changed the title [WIP] [12팀 배성규] [Chapter 2-2] 디자인 패턴과 함수형 프로그래밍 [12팀 배성규] [Chapter 2-2] 디자인 패턴과 함수형 프로그래밍 Jan 16, 2025
* @param cart
* @returns getLocalStorage / addLocalStorage / updateLocalStorage
*/
export const useLocalStorage = () => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이번주 테오 멘토링 시간에 배운건데 use를 사용하는 훅은 모두 react의 상태를 다루는 값들을 관리할때 사용되는거라고 하더라구요!
지금 useLocalStorage 도 보면 useLocalStorage자체적인 state를 가지고 있지않고,
getLocalStorage, addLocalStorage, updateLocalStorage 의 함수들이 유틸함수로 빠질수 있을것 같아보여요!

사실 저도 이부분에대해 고민을 많이했었는데 계속 고민중인 부분이라 같이한번 논의해보면 좋을것같습니다~!!

Comment on lines +8 to +13
}: {
item: CartItem;
appliedDiscount: number;
updateQuantity: (id: string, quantity: number) => void;
removeFromCart: (id: string) => void;
}) => (
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

동작은 동일하겠지만 interface로 빼두면 가독성이 개선될 것 같아요! props가 엄청 커지는 경우도 있어서 ㅎ.ㅎ

Comment on lines 24 to 37
const getRemainingStock = (product: Product) => {
const cartItem = cart.find(item => item.product.id === product.id);
const cartItem = cart.find((item) => item.product.id === product.id);
return product.stock - (cartItem?.quantity || 0);
};

const { totalBeforeDiscount, totalAfterDiscount, totalDiscount } = calculateTotal()

const getAppliedDiscount = (item: CartItem) => {
const { discounts } = item.product;
const { quantity } = item;
let appliedDiscount = 0;
for (const discount of discounts) {
if (quantity >= discount.quantity) {
appliedDiscount = Math.max(appliedDiscount, discount.rate);
}
}
return appliedDiscount;
return item.product.discounts.reduce(
(maxDiscount, discount) =>
item.quantity >= discount.quantity
? Math.max(maxDiscount, discount.rate)
: maxDiscount,
0
);
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요런 것도 순수함수로 만들어서 컴포넌트에서 분리한다면 좋을 것 같아요!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 찬성입니다!

Copy link

@Jayteeee Jayteeee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생많으셨습니다!~

* @todo setLocalStorage의 방식
* 1. addtocart -> 카트 추가할 때 로컬스트리지에 값을 추가한다 .
* 2. remove -> 카트 삭제될 때 로컬스토리지에 값을 제거한다.
* 3. update -> 카트 내부의 수량을 조절한다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

적절한 주석처리인거 같아요! 이해하기 좋은거 같습니다.

Comment on lines +20 to +82
return (
<div>
<h4 className="text-lg font-semibold mb-2">할인 정보</h4>
{editingProduct ? (
editingProduct.discounts.map((discount, index) => (
<div key={index} className="flex justify-between items-center mb-2">
<span>
{discount.quantity}개 이상 구매 시 {discount.rate * 100}% 할인
</span>
<button
onClick={() => handleRemoveDiscount(product.id, index)}
className="bg-red-500 text-white px-2 py-1 rounded hover:bg-red-600"
>
삭제
</button>
</div>
))
) : (
<div>
{product.discounts.map((discount, index) => (
<div key={index} className="mb-2">
<span>
{discount.quantity}개 이상 구매 시 {discount.rate * 100}% 할인
</span>
</div>
))}
</div>
)}
<div className="flex space-x-2">
<input
type="number"
placeholder="수량"
value={newDiscount.quantity}
onChange={(e) =>
setNewDiscount({
...newDiscount,
quantity: parseInt(e.target.value),
})
}
className="w-1/3 p-2 border rounded"
/>
<input
type="number"
placeholder="할인율 (%)"
value={newDiscount.rate * 100}
onChange={(e) =>
setNewDiscount({
...newDiscount,
rate: parseInt(e.target.value) / 100,
})
}
className="w-1/3 p-2 border rounded"
/>
<button
onClick={() => handleAddDiscount(product.id)}
className="w-1/3 bg-blue-500 text-white p-2 rounded hover:bg-blue-600"
>
할인 추가
</button>
</div>
</div>
);
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

취향 차이이긴 한데 이벤트 핸들러는 위쪽에 선언해서 사용하는 것 어떻게 생각하시나요!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위쪽으로 뺀다는게 어떤 말씀인지 이해를 못했습니다 ㅎ ㅎ ..

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

함수 자체를 return문 아래서 하는게 아닌 위에서 하는 방식입니다!
input을 예로 들자면 아래와 같은 친구를

(e) =>
  setNewDiscount({
  ...newDiscount,
  rate: parseInt(e.target.value) / 100,
})

상단에서

const onChnageDiscountRateInput = (e)=>{
 ... 함수로직
}

이런 방식으로 사용해서 아래에는

  <input
          type="number"
          placeholder="수량"
          value={newDiscount.quantity}
          onChange={onChnageDiscountRateInput}
          className="w-1/3 p-2 border rounded"
        />

이렇게 사용하는거죠! 어떻게 생각하시나요~

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아~ 무슨 말씀인지 이해했습니다!
따로 함수로 분리하니까 볼 때 더 깔끔해보이네요b 좋아보입니다!

Copy link

@hty0525 hty0525 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

전체적으로 깔끔합니다!
이번주차도 고생하셨습니다!!

export const useToggle = (initialState: boolean) => {
const [state, setState] = useState(initialState);
const toggle = () => setState((prev) => !prev);
return [state, toggle] as const;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 toggle도 커스텀훅으로 만드셨군요! 이 부분은 나중에 재사용하기에 되게 유용할 것 같아요 👍

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

토요일에 리뷰 달았었는데... 제가 설정을 잘못해서 저만 보였더라구요 ㅠㅠ 늦게나마 다시 올려요

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zz괜찮습니다~ 리뷰 감사합니다!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants